[s3] Avoid ListBucket requirement in snapshot commit path by hhhizzz · Pull Request #8263 · apache/paimon

hhhizzz · 2026-06-17T06:50:49Z

Purpose

Avoid requiring ListBucket for S3-compatible object-store snapshot commits, while preserving the stale snapshot conflict handling.

The change skips the exists(snapshot-N) probe on object stores, but writes snapshot-N with no-overwrite semantics. For S3 this uses If-None-Match: *, and maps 412 PreconditionFailed to FileAlreadyExistsException, so stale commits retry instead of overwriting existing snapshot metadata.

Non-object-store behavior is unchanged.

Tests

Added regression tests for:

object-store snapshot commit without probing exists(snapshot-N)
stale object-store commit conflict does not overwrite an existing snapshot-N
S3 no-overwrite snapshot write uses direct PUT
S3 412 PreconditionFailed is treated as the file-exists conflict signal

Verified with:

mvn -nsu -pl paimon-core -am -DskipITs -Dcheckstyle.skip -Dspotless.check.skip=true -Drat.skip=true -DfailIfNoTests=false -Dtest=RenamingSnapshotCommitTest,HintFileUtilsTest test
mvn -nsu -pl paimon-filesystems/paimon-s3-impl -am -DskipITs -Dcheckstyle.skip -Dspotless.check.skip=true -Drat.skip=true -DfailIfNoTests=false -Dtest=S3FileIOTest test

Also verified against Ceph RGW with a temporary user without ListBucket: first conditional PUT succeeds, second returns 412, and the original object

JingsongLi · 2026-06-19T00:58:45Z

I think this still breaks commit conflict handling on object stores. The latest snapshot is read before acquiring the commit lock, and newSnapshotPath is computed from that value. Previously, the !fileIO.exists(newSnapshotPath) check inside lock.runWithLock was the stale-snapshot guard: if another writer committed the same snapshot id while this writer was waiting for the lock, this returned false and FileStoreCommitImpl retried with the newer latest snapshot. With the new object-store branch, we skip that check and then write snapshot-N with overwrite=true, so two serialized object-store commits that both planned snapshot N can both return success, with the second one replacing the first snapshot file. That can lose a committed snapshot even when the external lock is working. We need to preserve the no-overwrite/conflict signal for an existing snapshot file without requiring ListBucket, rather than blindly overwriting snapshot metadata on object stores.

hhhizzz · 2026-06-19T02:23:53Z

Thanks for catching this.The previous object-store branch could skip the stale snapshot guard and overwrite an existing snapshot-N.

I updated the PR to keep the no-overwrite conflict signal without requiring ListBucket:

object stores still skip exists(snapshot-N)
snapshot-N is written with no-overwrite semantics
S3 uses If-None-Match: * for that write
412 PreconditionFailed is mapped to FileAlreadyExistsException, so the commit returns false and retries instead of overwriting

Added regression tests for the stale commit conflict path and S3 412 handling.

Verified with focused Maven tests, and with Ceph RGW using a temporary user without ListBucket: first conditional PUT succeeds, second returns 412, and the original object is preserved.

JingsongLi · 2026-06-20T03:30:59Z

+            // On object stores, probing a missing object can require list permission. Trust the
+            // committed hint and let the commit retry path handle a concurrently newer snapshot.
+            if (fileIO.isObjectStore()) {
+                return snapshotId;


This still falls back to listing when LATEST is absent (for example on the first commit of a new table, or after the hint file is lost). FileStoreCommitImpl calls snapshotManager.latestSnapshot() before it writes snapshot-1, so with an S3 user that has GetObject/PutObject but no ListBucket, an empty table will fail here before the new conditional PUT path is reached. The new object-store test writes LATEST first, so it does not cover this case. For object stores we need a no-list path for a missing hint as well, such as treating the missing hint as no latest snapshot and relying on the no-overwrite snapshot write to reject an unexpected existing snapshot-1.

hhhizzz · 2026-06-22T06:37:30Z

Updated the branch with the current no-list first-snapshot logic and additional conflict coverage.

The current behavior is:

For object stores whose FileIO supports atomic create-without-overwrite, snapshot metadata files are written with a no-overwrite create path. On S3 this is implemented with conditional PUT (If-None-Match: *).
If LATEST is readable, we validate the hinted snapshot file and fail closed if the hint is stale enough that snapshot-(latest + 1) already exists.
If LATEST is missing or hidden by no ListBucket access, and EARLIEST is also absent/unreadable, we treat this as no latest snapshot. The first commit then attempts to create snapshot-1 with conditional PUT.
If snapshot-1 already exists, the write does not overwrite it. Same content is treated as idempotent; different/malformed/unreadable content raises a conflict signal that requires latest-snapshot recovery by listing.
The normal commit loop and manifest compaction path now recover from that signal by listing and retrying. replaceManifestList maps it back to its existing false conflict result.
If recovery listing is denied, the exception explicitly says that ListBucket is required or a valid LATEST hint must be restored.
Non-object-store paths and FileIOs without no-overwrite support continue to use the original atomic rename / exists-based path.

I also explicitly checked the ambiguous damaged-metadata case: if both LATEST and EARLIEST are lost/unreadable and all low snapshot files such as snapshot-1 have already expired while later snapshots remain, that state is not distinguishable from a new table without ListBucket or another durable readable hint. This PR treats that as metadata-corruption / disaster recovery: restore hints or temporarily grant ListBucket.

Validation:

TDD regression:
- Verified the new direct-writer tests failed before the fix with SnapshotCommitConflictRequiresListRecoveryException.
- Fixed compactManifest and replaceManifestList, then verified the tests passed.
Unit tests:
- mvn -B -ntp -pl paimon-core -Dtest=HintFileUtilsTest,RenamingSnapshotCommitTest,FileStoreCommitTest#testSnapshotCommitConflictTriggersListRecoveryFromCommitLoop+testSnapshotCommitConflictDoesNotRecoverWhenRetryBudgetExhausted+testManifestCompactConflictTriggersListRecovery+testReplaceManifestListConflictReturnsFalse -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test
- Result: 25 tests, 0 failures/errors.
S3 FileIO tests:
- mvn -B -ntp -pl paimon-filesystems/paimon-s3-impl -am -Dtest=S3FileIOTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test
- Result: 11 tests, 0 failures/errors.
Ceph/RGW smoke:
- Created a temporary RGW user with only GetObject, PutObject, and DeleteObject under a fresh test prefix; no ListBucket.
- Verified AWS_LIST_DENIED=true.
- Added an admin-side precheck before the Paimon first commit: ADMIN_TABLE_KEYS_BEFORE=0, so the table prefix had no pre-existing objects or directory marker.
- Verified no-list first commit: LATEST_BEFORE=null, FIRST_COMMIT=true, LATEST_AFTER=1.
- Verified stale same-id conflict does not overwrite the committed snapshot: CONFLICT_REQUIRES_RECOVERY=true, SNAPSHOT_USER_AFTER_CONFLICT=first-user.
- Verified recovery listing is denied without ListBucket: RECOVERY_LIST_DENIED=true.
- Cleanup succeeded: RESTORE_POLICY=ok, DELETE_TEMP_USER=ok.

JingsongLi · 2026-06-23T05:44:57Z

+            Optional<Long> latestHint = readLatestHintForNoListObjectStore(fileIO, LATEST, dir);
+            Long snapshotId = latestHint.orElse(null);
+            if (supportsNoListSnapshotCommit(fileIO, file, snapshotId)) {
+                return snapshotId == null


This no-list branch treats a missing LATEST hint as an empty table whenever EARLIEST is also absent. A normal table with existing snapshots can be in that state: snapshot commits only update LATEST, and EARLIEST is only written later by expiration/rollback paths. If the LATEST object is accidentally deleted or temporarily unreadable (the S3 no-ListBucket case this PR is handling), latestSnapshotId() now returns null instead of falling back to listing or failing closed, so readers/committers can treat a non-empty table as new and attempt snapshot-1 again. We should not return null solely because both hints are missing; either verify snapshot-1 is absent using the conditional-create/read path, fall back to list, or fail with the same recovery-required message.

JingsongLi reviewed Jun 20, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

hhhizzz marked this pull request as draft June 21, 2026 16:13

Qiwei Huang added 11 commits June 22, 2026 00:44

[s3] Add atomic no-overwrite file IO capability

88c8e85

[s3] Use conditional PUT for snapshot metadata

01d5547

[s3] Tighten conditional PUT conflict mapping

5ccecdd

[s3] Add no-list latest snapshot lookup mode

d5a60fb

[s3] Avoid exists probe for no-list latest hint reads

a0c62e9

[s3] Preserve no-overwrite snapshot conflict signal

2c57144

[s3] Recover latest snapshot by listing after no-overwrite conflict

27705b7

[s3] Check retry budget before snapshot recovery listing

8eddca9

[s3] Treat no-list missing latest hint as absent

4a9b1cf

[s3] Fail closed for stale no-list latest hint

db3e4e4

[s3] Recover direct snapshot conflicts consistently

cd6b31f

hhhizzz force-pushed the fix-s3-snapshot-access branch from 4ce509a to cd6b31f Compare June 22, 2026 04:43

hhhizzz marked this pull request as ready for review June 22, 2026 06:37

JingsongLi reviewed Jun 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[s3] Avoid ListBucket requirement in snapshot commit path#8263

[s3] Avoid ListBucket requirement in snapshot commit path#8263
hhhizzz wants to merge 11 commits into
apache:masterfrom
hhhizzz:fix-s3-snapshot-access

hhhizzz commented Jun 17, 2026 •

edited

Loading

Uh oh!

JingsongLi commented Jun 19, 2026

Uh oh!

hhhizzz commented Jun 19, 2026

Uh oh!

JingsongLi Jun 20, 2026

Uh oh!

This comment was marked as outdated.

hhhizzz commented Jun 22, 2026

Uh oh!

JingsongLi Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hhhizzz commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Tests

Uh oh!

JingsongLi commented Jun 19, 2026

Uh oh!

hhhizzz commented Jun 19, 2026

Uh oh!

JingsongLi Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

hhhizzz commented Jun 22, 2026

Uh oh!

JingsongLi Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hhhizzz commented Jun 17, 2026 •

edited

Loading